-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change E0369 to give note informations for foreign items. #126925
Conversation
Make it easy for developers to understand why the binop cannot be applied. fixes rust-lang#125631
I searched and finded it seems that foreign item define cannot be displayed correctly in For: use std::io::{Error, ErrorKind};
use std::thread;
struct T1;
struct T2;
fn main() {
(Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2)
== (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2);
//~^ERROR binary operation `==` cannot be applied to type
} error[E0369]: binary operation `==` cannot be applied to type `(std::io::Error, Thread, T1, T2)`
--> ./src/main.rs:221:9
|
220 | (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2)
| -------------------------------------------------------------- (std::io::Error, Thread, T1, T2)
221 | == (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2);
| ^^ -------------------------------------------------------------- (std::io::Error, Thread, T1, T2)
|
note: the following types would have to `impl` their required traits for this operation to be valid
--> ./src/main.rs:211:1
|
211 | struct T1;
| ^^^^^^^^^ must implement `PartialEq`
212 | struct T2;
| ^^^^^^^^^ must implement `PartialEq`
note: the foreign item types don't implement required traits for this operation to be valid
--> D:\source\rust\rust\library\std\src\io\error.rs:67:1
|
67 | pub struct Error {
| ^^^^^^^^^^^^^^^^ not implement `PartialEq`
|
::: D:\source\rust\rust\library\std\src\thread\mod.rs:1313:1
|
1313 | pub struct Thread {
| ^^^^^^^^^^^^^^^^^ not implement `PartialEq`
help: consider annotating `T1` with `#[derive(PartialEq)]`
|
211 + #[derive(PartialEq)]
212 | struct T1;
|
help: consider annotating `T2` with `#[derive(PartialEq)]`
|
212 + #[derive(PartialEq)]
213 | struct T2;
|
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0369`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the message is really redundant. What is this PR trying to achieve?
The correct solution to #125631 IMO is that we should detect when we have diagnostic::on_unimplemented
/rustc_on_unimplemented
overwriting the default trait unimplemented message, and add the old primary message as a note if so.
In my opinion, there's nothing special about the Adt
types, and this has nothing to do with foreign types or spans either?
@@ -5,6 +5,11 @@ LL | fn main() { let x = "a".to_string() ^ "b".to_string(); } | |||
| --------------- ^ --------------- String | |||
| | | |||
| String | |||
| | |||
note: the foreign item type `String` doesn't implement `BitXor` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This, and most of the other messages that this PR adds, feels really redundant.
@rustbot author |
Thank you very much. As the example in #125631, for foreign items, E0369 will not show details information about My initial thought was that for foreign items, we cann't require them to implement these traits to satisfy the binop operation, so I separated the local and foreign items, and just use a note to explain the reasons for the foreign items. For: For: I don’t understand the mechanism you’re talking about yet. Could you give me more details? |
What does this have to do with foreign (non-local) types? I don't see anything in that issue that discusses that this is an limitation of non-local types. |
Oh --
I don't actually think this is true? If it is, we should find the code that is filtering local types, rather than just adding a new note. Otherwise, this needs more investigation. I don't think we need to add a new note here. |
In his |
Can you come up an example where this note gets produced for a local type? I would then recommend using that example to find out how to fix that to make it produce the right note, rather than just adding a new one. |
OK. Thank you. |
This PR only involves this part about note, Not including the following message about help: note: the following types would have to `impl` their required traits for this operation to be valid
--> src/main.rs:1:1
|
1 | struct T1;
| ^^^^^^^^^ must implement `PartialEq`
2 | struct T2;
| ^^^^^^^^^ must implement `PartialEq` |
Actually, nevermind. I think I understand the approach of this PR. |
@bors r+ |
Thank you. |
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#126724 (Fix a span in `parse_ty_bare_fn`.) - rust-lang#126812 (Rename `tcx` to `cx` in new solver generic code) - rust-lang#126879 (fix Drop items getting leaked in Filter::next_chunk) - rust-lang#126925 (Change E0369 to give note informations for foreign items.) - rust-lang#126938 (miri: make sure we can find link_section statics even for the local crate) - rust-lang#126954 (resolve: Tweak some naming around import ambiguities) - rust-lang#126964 (Migrate `lto-empty`, `invalid-so` and `issue-20626` `run-make` tests to rmake.rs) - rust-lang#126968 (Don't ICE during RPITIT refinement checking for resolution errors after normalization) - rust-lang#126971 (Bump black, ruff and platformdirs) - rust-lang#126973 (Fix bad replacement for unsafe extern block suggestion) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#126925 - surechen:fix_125631, r=compiler-errors Change E0369 to give note informations for foreign items. Change E0369 to give note informations for foreign items. Make it easy for developers to understand why the binop cannot be applied. fixes rust-lang#125631
Change E0369 to give note informations for foreign items.
Make it easy for developers to understand why the binop cannot be applied.
fixes #125631